Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

XLOOKUP function #1469

Draft
wants to merge 23 commits into
base: develop
Choose a base branch
from
Draft

XLOOKUP function #1469

wants to merge 23 commits into from

Conversation

sequba
Copy link
Contributor

@sequba sequba commented Dec 5, 2024 •
edited
Loading

Context

  • merge PR by @selimyoussry
  • add more unit tests including several edge cases
  • adjust the implementation to meet the criteria specified by the tests
  • add changelog entry
  • add entry to the built-in functions page
  • describe the limitation of the function and differences between HyperFormula and Excel and Google Sheets

How did you test your changes?

unit tests

Types of changes

  • Breaking change (a fix or a feature because of which an existing functionality doesn't work as expected anymore)
  • New feature or improvement (a non-breaking change that adds functionality)
  • Bug fix (a non-breaking change that fixes an issue)
  • Additional language file, or a change to an existing language file (translations)
  • Change to the documentation

Related issues:

Fixes #1458

Checklist:

  • I have reviewed the guidelines about Contributing to HyperFormula and I confirm that my code follows the code style of this project.
  • I have signed the Contributor License Agreement.
  • My change is compliant with the OpenDocument standard.
  • My change is compatible with Microsoft Excel.
  • My change is compatible with Google Sheets.
  • I described my changes in the CHANGELOG.md file.
  • My changes require a documentation update.
  • My changes require a migration guide.

selimyoussry and others added 16 commits June 15, 2024 13:39
@selimyoussry
ISS-1 sapiologie/hyperformula XLOOKUP in built-in-functions.md
932e8c0
@selimyoussry
ISS-1 sapiologie/hyperformula add translations
f0f9545
@selimyoussry
ISS-1 sapiologie/hyperformula placeholder function and test
384bcb8
@selimyoussry
ISS-1 sapiologie/hyperformula progress on Xlookup logic
eecce3a
@selimyoussry
ISS-1 sapiologie/hyperformula Working XLOOKUP without support for opt…
e9cabd6 
…ional parameters
@selimyoussry
ISS-1 sapiologie/hyperformula override default if_not_found
83be86c
@selimyoussry
ISS-1 sapiologie/hyperformula add commented out range function return…
5101a90 
… test
@selimyoussry
ISS-1 sapiologie/hyperformula update the doc to reflect limitations
08445d1
@selimyoussry
ISS-1 sapiologie/hyperformula Set vectorizationForbidden: true on XLO…
9ece67c 
…OKUP
@sequba
Add test suite for the XLOOKUP function
9c448c2
@sequba
Plan basic tests for XLOOKUP
167cac8
@sequba
Merge branch 'iss-1-xlookup' of github.com:sapiologie/hyperformula in…
1f9d091 
…to sapiologie-iss-1-xlookup
@sequba
Merge branch 'sapiologie-iss-1-xlookup' into feature/issue-1458
0811866
@sequba
Make XLOOKUP work in basic mode
8699958
@sequba
Make XLOOKUP return a range
13b9cc0
@sequba
Add unit tests about the types of argument
d3c6a26
@sequba sequba linked an issue Dec 5, 2024 that may be closed by this pull request
Add XLOOKUP function #1458
Open
@sequba sequba self-assigned this Dec 5, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2024 •
edited
Loading

Performance comparison of head (bfac296) vs base (bf325c3)

                                     testName |   base |   head | change
------------------------------------------------------------------------
                                      Sheet A | 511.14 | 510.19 | -0.19%
                                      Sheet B | 164.71 | 163.17 | -0.93%
                                      Sheet T | 143.86 | 142.85 | -0.70%
                                Column ranges | 508.73 | 508.65 | -0.02%
Sheet A:  change value, add/remove row/column |  13.32 |  14.24 | +6.91%
 Sheet B: change value, add/remove row/column | 131.02 | 129.35 | -1.27%
                   Column ranges - add column | 146.59 |  151.5 | +3.35%
                Column ranges - without batch | 432.63 | 442.74 | +2.34%
                        Column ranges - batch |  110.3 | 114.09 | +3.44%

@codecov Codecov
Copy link

codecov bot commented Dec 7, 2024

Codecov Report

Attention: Patch coverage is 90.32258% with 6 lines in your changes missing coverage. Please review.

Project coverage is 97.17%. Comparing base (bf325c3) to head (af99365).

Files with missing lines Patch % Lines
src/interpreter/plugin/LookupPlugin.ts 90.32% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1469      +/-   ##
===========================================
- Coverage    97.20%   97.17%   -0.04%     
===========================================
  Files          169      169              
  Lines        14430    14484      +54     
  Branches      3027     3050      +23     
===========================================
+ Hits         14027    14075      +48     
- Misses         403      409       +6
Files with missing lines Coverage Δ
src/i18n/languages/csCZ.ts 100.00% <ø> (ø)
src/i18n/languages/daDK.ts 100.00% <ø> (ø)
src/i18n/languages/deDE.ts 100.00% <ø> (ø)
src/i18n/languages/enGB.ts 100.00% <ø> (ø)
src/i18n/languages/esES.ts 100.00% <ø> (ø)
src/i18n/languages/fiFI.ts 100.00% <ø> (ø)
src/i18n/languages/frFR.ts 100.00% <ø> (ø)
src/i18n/languages/huHU.ts 100.00% <ø> (ø)
src/i18n/languages/itIT.ts 100.00% <ø> (ø)
src/i18n/languages/nbNO.ts 100.00% <ø> (ø)
... and 7 more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sequba sequba

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add XLOOKUP function
2 participants
@sequba @selimyoussry